Skip to content

fix(scheduler): drain owned tasks before shutdown - #671

Open
Kuang-xianxin wants to merge 1 commit into
taskiq-python:masterfrom
Kuang-xianxin:fix/scheduler-cancellation-cleanup
Open

Kuang-xianxin wants to merge 1 commit into
taskiq-python:masterfrom
Kuang-xianxin:fix/scheduler-cancellation-cleanup

Conversation

@Kuang-xianxin

Copy link
Copy Markdown

Cancelling SchedulerLoop.run() currently leaves its scheduled sends and background schedule refresh running. Through the CLI, run_scheduler() proceeds to shut down the broker and schedule sources while those operations still use them. Through run_scheduler_task(), the orphaned operations can continue in the application's event loop after the caller has awaited cancellation.

Cancel and await the loop's owned sends and refresh in finally, before returning control to the caller. Ignore a cancelled refresh in its completion callback so normal shutdown does not raise CancelledError through the event-loop exception handler. The existing CLI/API cancellation behavior is preserved.

The regression test drives the loop, API and CLI entry points with event-controlled broker sends and source reads. Both operations block again during asynchronous cleanup: the test verifies that the parent stays pending and shutdown hooks do not run until cleanup is explicitly released. It also checks final cancellation state and absence of callback errors. All three cases fail on the unmodified implementation.

Validation: Windows / CPython 3.13.1, dependencies from uv.lock with all extras; pytest -q --tb=short: 349 passed (including 64 scheduler/API tests). No live external broker was used.

All applicable pre-commit hooks pass on both changed files, including the repository-wide Ruff and mypy checks. The --all-files secret scan also flags three existing strings in docs/README.md on Windows; running that hook on an untouched c4b8666 worktree produces the same findings. The changed files pass that hook.

This is separate from #627's optional send timeout: it handles tasks owned by a loop that is exiting and does not introduce a timeout or alter normal dispatch scheduling. It also does not change signal handling discussed in #461.

@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.96%. Comparing base (c4b8666) to head (3c89db1).
⚠️ Report is 19 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #671      +/-   ##
==========================================
+ Coverage   82.63%   82.96%   +0.32%     
==========================================
  Files          69       69              
  Lines        2730     2741      +11     
==========================================
+ Hits         2256     2274      +18     
+ Misses        474      467       -7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant